Add ability to handle events that expect an ack #463
Conversation
|
I didn't add any tests because I didn't have the time to do it yet. But I've tested locally with my usecase and it seems to work |
| @@ -25,7 +25,7 @@ pub struct Packet { | |||
| pub packet_type: PacketId, | |||
| pub nsp: String, | |||
| pub data: Option<String>, | |||
| pub id: Option<i32>, | |||
| pub id: Option<AckId>, | |||
There was a problem hiding this comment.
While I'm generally a fan of wrapper types, it's not worth the breaking API change.
There was a problem hiding this comment.
the reason I put this in is just so you can't make a mistake when calling ack on the client. Since you're not supposed to care what the ack id is I think this is a good change and it's better to make this change sooner rather than later. If you still disagree I can drop it
There was a problem hiding this comment.
I agree it's a reasonable change, but it would need to be made (and merged) separately.
I could not figure out how to run the |
mendess
force-pushed
the
mendess/ack-server-request
branch
from
b5935fb
to
034578f
Compare
agreed, that target is a little lacking. You need to start up the test containers beforehand as seen in the ci: https://github.com/1c3t3a/rust-socketio/blob/2ef32ecbe053d100e350c4d77a71dd1cab19471e/.github/workflows/test.yml#L36C11-L37C133, speaking of which I'll turn that on for you (it'll run on every push if you can't get it running locally) |
mendess
force-pushed
the
mendess/ack-server-request
branch
from
034578f
to
a290bc2
Compare
This adds three new methods to the API surface
[Raw]Client::ack(AckId, D)ClientBuilder::on_with_ack(Event, callback)ClientBuilder::on_any_with_ack(Event, callback)using these it's possible to receive messages that require ack and acknowledge them
Fixes #461